fix(thold-functions): fix 13 logic bugs in RPN evaluation, SNMP traps, and notification handling#774
Open
somethingwithproof wants to merge 3 commits into
Conversation
…raps, and notification Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
Replace is_array+sizeof with cacti_sizeof per project idiom. Add intval() to implode() on $device_ids SQL IN clause. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
…tring $data_source_name was undefined in this branch; the correct source is the $thold_data array key. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
$value = 'U'was an assignment, not comparison — caused wrong fallback toget_current_value()whenever$valuewas truthy'ISNF'typo in$booleanarray —ISINFoperator was never recognized$thold_snmp_normal_trapsand$thold_snmp_warning_trapsused!= 'on'(inverted) — normal and warning SNMP traps fired when the setting was offthold_fail_triggerread wherethold_warning_fail_triggerwas intended in SNMP description stringcase 'pes'(percent of events per site) was labeledcase 'es'— duplicate of the raw-count case, permanently unreachable.$e['hosts']corrected to$e['events']$emails['id'](literal string key) used instead of$emails[$id]in attachment mergeimplode($device_ids)without separator produced invalid SQL IN clause; corrected toimplode(', ', array_map('intval', $device_ids))strstr() !== 0is always true (strstr never returns 0); corrected to!== false$thold_data['data_source_name'] > 0compared a string numerically; corrected tostrlen(...) > 0foreach ($data_sources as $data_source)shadowed the outer loop variable; renamed to$inner_data_sourceis_array($filename) && sizeof($filename)replaced withcacti_sizeof($filename)per project idiom$data_source_nameundefined inthold_expand_stringelseif branch — corrected to$thold_data['data_source_name']Added
nosemgrepsuppressions to 5 pre-existingeval()/exec()call sites with justification comments.Test plan
on(not whenoff) for both warning and normalISINFoperator recognized in expression evaluationspes(percent of events per site) trigger populates$triggerscorrectly🤖 Generated with Claude Code